NO-ISSUE: Remove orphan files#3460
NO-ISSUE: Remove orphan files#3460openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRefactors cluster detail utilities by extracting helper functions into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx (1)
5-19: Simplify helpers by returning expressions directlyBoth helpers are correct, but removing temporary mutable variables will make the code shorter and easier to scan.
♻️ Proposed refactor
export const getNetworkType = (clusterNetworkType: Cluster['networkType']): string => { - let networkType: string; - clusterNetworkType === NETWORK_TYPE_SDN - ? (networkType = 'Software-Defined Networking (SDN)') - : (networkType = 'Open Virtual Network (OVN)'); - return networkType; + return clusterNetworkType === NETWORK_TYPE_SDN + ? 'Software-Defined Networking (SDN)' + : 'Open Virtual Network (OVN)'; }; export const getManagementType = ({ userManagedNetworking }: Cluster): string => { - let managementType: string; - userManagedNetworking - ? (managementType = 'User-Managed Networking') - : (managementType = 'Cluster-managed networking'); - return managementType; + return userManagedNetworking ? 'User-Managed Networking' : 'Cluster-managed networking'; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx` around lines 5 - 19, Replace the temporary mutable variables in getNetworkType and getManagementType with direct expression returns: for getNetworkType return the ternary expression based on clusterNetworkType (comparing to NETWORK_TYPE_SDN) instead of assigning to networkType, and for getManagementType return the ternary expression based on the Cluster's userManagedNetworking property instead of assigning to managementType; update only those functions (getNetworkType, getManagementType) to return the expressions directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx`:
- Around line 5-19: Replace the temporary mutable variables in getNetworkType
and getManagementType with direct expression returns: for getNetworkType return
the ternary expression based on clusterNetworkType (comparing to
NETWORK_TYPE_SDN) instead of assigning to networkType, and for getManagementType
return the ternary expression based on the Cluster's userManagedNetworking
property instead of assigning to managementType; update only those functions
(getNetworkType, getManagementType) to return the expressions directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0bc004a3-8b16-416e-9e01-c87cca2c4a5c
📒 Files selected for processing (8)
libs/locales/lib/en/translation.jsonlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterPropertiesUtils.test.tslibs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/index.ts
💤 Files with no reviewable changes (4)
- libs/ui-lib/lib/ocm/components/clusterDetail/index.ts
- libs/locales/lib/en/translation.json
- libs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx (1)
5-11: Consider simplifying to a direct ternary return.The intermediate variable and assignment-in-ternary pattern is verbose. A direct return is cleaner and more idiomatic.
♻️ Suggested simplification
export const getNetworkType = (clusterNetworkType: Cluster['networkType']): string => { - let networkType: string; - clusterNetworkType === NETWORK_TYPE_SDN - ? (networkType = 'Software-Defined Networking (SDN)') - : (networkType = 'Open Virtual Network (OVN)'); - return networkType; + return clusterNetworkType === NETWORK_TYPE_SDN + ? 'Software-Defined Networking (SDN)' + : 'Open Virtual Network (OVN)'; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx` around lines 5 - 11, The function getNetworkType currently uses an intermediate variable and assignment inside a ternary; simplify it to return the string directly from the ternary expression using the Cluster['networkType'] parameter and the NETWORK_TYPE_SDN constant (i.e., replace the variable + ternary-assignment pattern with a single return clusterNetworkType === NETWORK_TYPE_SDN ? 'Software-Defined Networking (SDN)' : 'Open Virtual Network (OVN)').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx`:
- Around line 13-19: The getManagementType function returns inconsistent label
capitalization; update the two returned strings so they match (e.g., change
"Cluster-managed networking" to "Cluster-Managed Networking" to match
"User-Managed Networking") by editing the string literals in getManagementType
(which reads the Cluster.userManagedNetworking flag) so both labels use the same
title-case style.
- Around line 24-44: The getDiskEncryptionEnabledOnStatus function currently
only handles 'all', 'masters', and 'workers' and returns null for other
DiskEncryption['enableOn'] values; update this function to add switch cases for
'arbiters' (render "Enabled on arbiters"), 'masters,arbiters' (render "Enabled
on control plane nodes and arbiters"), 'masters,workers' (render "Enabled on
control plane nodes and workers"), and 'arbiters,workers' (render "Enabled on
arbiters and workers") so the disk encryption row is shown for those
combinations; use the existing JSX pattern (fragments and <br /> where needed)
within getDiskEncryptionEnabledOnStatus to match the current UI style.
---
Nitpick comments:
In `@libs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsx`:
- Around line 5-11: The function getNetworkType currently uses an intermediate
variable and assignment inside a ternary; simplify it to return the string
directly from the ternary expression using the Cluster['networkType'] parameter
and the NETWORK_TYPE_SDN constant (i.e., replace the variable +
ternary-assignment pattern with a single return clusterNetworkType ===
NETWORK_TYPE_SDN ? 'Software-Defined Networking (SDN)' : 'Open Virtual Network
(OVN)').
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: edda31f6-2b22-4bb6-ba88-075c3ec9b057
📒 Files selected for processing (8)
libs/locales/lib/en/translation.jsonlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterPropertiesUtils.test.tslibs/ui-lib/lib/ocm/components/clusterDetail/clusterPropertiesUtils.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/index.ts
💤 Files with no reviewable changes (4)
- libs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsx
- libs/locales/lib/en/translation.json
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterPropertiesUtils.test.ts
|
@LiorSoffer: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.tsx (1)
17-54:'all'disk-encryption label should include arbiters.
case 'all'currently renders only control plane + workers, but other branches treat arbiters as a distinct role. This can under-report coverage in the UI when arbiters exist.Proposed alignment fix
- case 'all': - diskEncryptionType = ( - <> - Enabled on control plane nodes - <br /> - Enabled on workers - </> - ); - break; + case 'all': + case 'masters,arbiters,workers': + diskEncryptionType = ( + <> + Enabled on control plane nodes + <br /> + Enabled on arbiters + <br /> + Enabled on workers + </> + ); + break; @@ - case 'masters,arbiters,workers': - diskEncryptionType = ( - <> - Enabled on control plane nodes - <br /> - Enabled on arbiters - <br /> - Enabled on workers - </> - ); - break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.tsx` around lines 17 - 54, The 'all' branch of the switch that sets diskEncryptionType only mentions control plane nodes and workers and must also mention arbiters to match other cases; update the switch case 'all' (variable diskEncryptionType) to include "Enabled on arbiters" (with the same line-break formatting used in the 'masters,arbiters,workers' case) so the UI accurately reports disk encryption for arbiters when all roles are selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.tsx`:
- Around line 17-54: The 'all' branch of the switch that sets diskEncryptionType
only mentions control plane nodes and workers and must also mention arbiters to
match other cases; update the switch case 'all' (variable diskEncryptionType) to
include "Enabled on arbiters" (with the same line-break formatting used in the
'masters,arbiters,workers' case) so the UI accurately reports disk encryption
for arbiters when all roles are selected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7498690-b074-4624-b424-974aee1cdbfb
📒 Files selected for processing (12)
.cursor/rules/patternfly-ux-capitalization.mdclibs/locales/lib/en/translation.jsonlibs/ui-lib-tests/cypress/support/variables/networking.tslibs/ui-lib/lib/common/components/clusterWizard/networkingSteps/ManagedNetworkingControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.test.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/index.tslibs/ui-lib/lib/ocm/components/featureSupportLevels/featureStateUtils.ts
💤 Files with no reviewable changes (3)
- libs/ui-lib/lib/ocm/components/clusterDetail/index.ts
- libs/ui-lib/lib/ocm/components/clusterDetail/AssistedInstallerExtraDetailCard.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
✅ Files skipped from review due to trivial changes (7)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewClusterDetailTable.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/utils.test.ts
- libs/ui-lib-tests/cypress/support/variables/networking.ts
- libs/ui-lib/lib/ocm/components/featureSupportLevels/featureStateUtils.ts
- libs/ui-lib/lib/common/components/clusterWizard/networkingSteps/ManagedNetworkingControlGroup.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- .cursor/rules/patternfly-ux-capitalization.mdc
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/locales/lib/en/translation.json
Signed-off-by: Lior Soffer <liorsoffer1@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgyselov, LiorSoffer The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d900a25
into
openshift-assisted:master
Summary by CodeRabbit